-
Notifications
You must be signed in to change notification settings - Fork 1
Improve Tracing #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: remote-vtables
Are you sure you want to change the base?
Conversation
| import org.apache.cassandra.distributed.api.IMessageFilters; | ||
| import org.apache.cassandra.distributed.api.Row; | ||
| import org.apache.cassandra.distributed.api.SimpleQueryResult; | ||
| import org.apache.cassandra.distributed.shared.Metrics; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: looks like an unnecessary import
| return added; | ||
| } | ||
|
|
||
| if (++bucketSeen < 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to increment once again here given we have just incremented on L648?
| if (intersects != null && (participants == null || !intersects.intersects(participants))) | ||
| return false; | ||
|
|
||
| return chance >= 1.0f || ThreadLocalRandom.current().nextFloat() > chance; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be ThreadLocalRandom.current().nextFloat() <= chance. For example, if we say chance is 0.95 (i.e. there is 95% chance of hitting this), we should return matches for 1.0 (and above), but also anything between 0 and 0.95
| { | ||
| if (keys != 0) out.append(','); | ||
| if (ranges == ALL_KINDS) out.append("R*"); | ||
| else TinyEnumSet.append(keys, Txn.Kind::forOrdinal, k -> "R" + k.shortName(), out); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be TinyEnumSet.append(ranges?
|
|
||
| cur = new TxnEvents(); | ||
| if (newBucketSubSize < 0) | ||
| cur.bucketSubSize = newBucketSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we always set bucket sub size to new bucket size if newBucketjsubSize < 0, and not only when cur == null?
In other words, move this if outside of cur == null if statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 means the user hasn't specified a value, but if we aren't inserting we don't need to update it (since there will already be some value). this is just for initialising to a default value
12d3157 to
0517436
Compare
fdaaba6 to
80322a9
Compare
b61f3cb to
5888fc1
Compare
…irtual tables from any node in the cluster patch by Benedict; reviewed by Alex Petrov for CASSANDRA-20900
baa8f18 to
0ec190b
Compare
|
good catches! |
- Introduce pattern tracing, that can intercept failed or new coordinations matching various filters
- Support additional tracing event collection modes (SAMPLE and RING)
patch by Benedict; reviewed by Alex Petrov for CASSANDRA-20911
0ec190b to
c2dc7f7
Compare
Opening up a separate PR since original tracing PR contains several other commits.